Test: Implemented OOP approach for tested applications in the pytest framework #1304
Conversation
awilczyns
force-pushed
the
test-framework-refactor-rxtxapp
branch
7 times, most recently
from
73b77f9 to
e77427c
Compare
Add parent class for common parameters and execution logic across RxTxApp, FFmpeg, and Gstreamer. Includes RxTxApp child class implementation. Signed-off-by: Wilczynski, Andrzej <[email protected]>
awilczyns
force-pushed
the
test-framework-refactor-rxtxapp
branch
6 times, most recently
from
c225d8a to
1db3b68
Compare
awilczyns
requested review from
DawidWesierski4,
Falron98 and
Sakoram
as code owners
…OOP approach. Signed-off-by: Wilczynski, Andrzej <[email protected]>
…OOP approach. Signed-off-by: Wilczynski, Andrzej <[email protected]>
| "video": "rawvideo", | ||
| "audio": "pcm_s24le", |
There was a problem hiding this comment.
nope
The names are wrong :<
There is no "comparison" between the audio st20 (without pipeline) in ffmpeg
types should be none
| "video": "mtl_video", | ||
| "audio": "mtl_audio", |
There was a problem hiding this comment.
doesn't exist like in ffmpeg
| FRAMERATE_TO_VIDEO_FORMAT_MAP = { | ||
| "p60": "i1080p60", | ||
| "p59": "i1080p59", | ||
| "p50": "i1080p50", | ||
| "p30": "i1080p30", | ||
| "p29": "i1080p29", | ||
| "p25": "i1080p25", | ||
| "p24": "i1080p24", | ||
| "p23": "i1080p23", | ||
| } |
There was a problem hiding this comment.
Not used anywhere?
I also don't think it's right
Where do we have such strings ? "i1080p29" ?
|
|
||
| # Default network configuration values | ||
| DEFAULT_NETWORK_CONFIG = { | ||
| "nic_port": "0000:31:01.0", |
There was a problem hiding this comment.
i wouldn't setthe default NIC port
It's gonna be kinda hard to debug when this pops up,
I would leave this as None.
We have a mechanism for getting the PCI port in this framework
I think this could be confusing
Feel free to disagree with me
| "st20p_port": 20000, | ||
| "st22p_port": 20000, | ||
| "st30p_port": 30000, | ||
| "video_port": 20000, | ||
| "audio_port": 30000, | ||
| "ancillary_port": 40000, | ||
| "fastmetadata_port": 40000, |
There was a problem hiding this comment.
We have so many ports :< Please separate all types :( It's def. gonna be easier down the line
Why repeat the ports set us up for failure?
| RXTXAPP_PARAM_MAP = { | ||
| # Network parameters | ||
| "source_ip": "ip", | ||
| "destination_ip": "dip", |
There was a problem hiding this comment.
lets use "ip",
it's gonna be easier and both work
"dip" and "ip",
"sip" was replaced with "ip" too
| # Maps universal parameter names to application-specific names | ||
|
|
||
| # RxTxApp parameter mapping | ||
| RXTXAPP_PARAM_MAP = { |
There was a problem hiding this comment.
we have a mix of command line arguments and input config parameters here is that fine ?
its can be a little hard to implement?
im fine with this if you are fine with this
| config_file_path = os.path.abspath( | ||
| DEFAULT_NETWORK_CONFIG["default_config_file"] |
There was a problem hiding this comment.
this will not work on other hosts ?
| log_dir = os.path.join(os.getcwd(), LOG_FOLDER, "latest") | ||
| os.makedirs(log_dir, exist_ok=True) | ||
| out_file_url = os.path.join(log_dir, "out.yuv") | ||
| host = list(hosts.values())[0] |
There was a problem hiding this comment.
again other not supported on other hosts
| os.path.join(media, media_file_info["filename"]) | ||
| if media | ||
| else media_file_info["filename"] | ||
| ) | ||
| # Ensure kahawai.json (plugin configuration) is available in build cwd so st22 encoder can load plugins | ||
| kahawai_src = os.path.abspath( | ||
| os.path.join(os.path.dirname(__file__), "../../../../../..", "kahawai.json") | ||
| ) | ||
| kahawai_dst = os.path.join(build, "kahawai.json") | ||
| try: | ||
| if os.path.exists(kahawai_src) and not os.path.exists(kahawai_dst): |
There was a problem hiding this comment.
os not supported on other hosts
Added parent class for common parameters and execution logic across RxTxApp, FFmpeg, and Gstreamer. Includes RxTxApp child class implementation.